Merge "build: Remove failing HHVM test from Travis CI config"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 8 Mar 2019 01:28:38 +0000 (01:28 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 8 Mar 2019 01:28:38 +0000 (01:28 +0000)
28 files changed:
includes/Storage/SqlBlobStore.php
includes/api/ApiStashEdit.php
includes/libs/objectcache/APCBagOStuff.php
includes/libs/objectcache/APCUBagOStuff.php
includes/libs/objectcache/BagOStuff.php
includes/libs/objectcache/CachedBagOStuff.php
includes/libs/objectcache/EmptyBagOStuff.php
includes/libs/objectcache/HashBagOStuff.php
includes/libs/objectcache/MemcachedBagOStuff.php
includes/libs/objectcache/MemcachedPeclBagOStuff.php
includes/libs/objectcache/MultiWriteBagOStuff.php
includes/libs/objectcache/RESTBagOStuff.php
includes/libs/objectcache/RedisBagOStuff.php
includes/libs/objectcache/ReplicatedBagOStuff.php
includes/libs/objectcache/WinCacheBagOStuff.php
includes/libs/rdbms/exception/DBReplicationWaitError.php
includes/objectcache/SqlBagOStuff.php
includes/pager/IndexPager.php
includes/pager/RangeChronologicalPager.php
includes/pager/ReverseChronologicalPager.php
includes/pager/TablePager.php
includes/specials/pagers/ActiveUsersPager.php
includes/specials/pagers/AllMessagesTablePager.php
includes/specials/pagers/ContribsPager.php
includes/specials/pagers/DeletedContribsPager.php
includes/specials/pagers/ImageListPager.php
tests/phpunit/includes/api/ApiStashEditTest.php
tests/phpunit/includes/user/UserTest.php

index d7216c5..a5c5985 100644 (file)
@@ -35,7 +35,6 @@ use Language;
 use MWException;
 use WANObjectCache;
 use Wikimedia\Assert\Assert;
-use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\LoadBalancer;
 
@@ -275,9 +274,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                        $this->getCacheKey( $blobAddress ),
                        $this->getCacheTTL(),
                        function ( $unused, &$ttl, &$setOpts ) use ( $blobAddress, $queryFlags ) {
-                               list( $index ) = DBAccessObjectUtils::getDBOptions( $queryFlags );
-                               $setOpts += Database::getCacheSetOptions( $this->getDBConnection( $index ) );
-
+                               // Ignore $setOpts; blobs are immutable and negatives are not cached
                                return $this->fetchBlob( $blobAddress, $queryFlags );
                        },
                        [ 'pcGroup' => self::TEXT_CACHE_GROUP, 'pcTTL' => IExpiringStore::TTL_PROC_LONG ]
@@ -417,7 +414,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                                        $this->getCacheKey( $cacheKey ),
                                        $this->getCacheTTL(),
                                        function () use ( $url, $flags ) {
-                                               // No negative caching per BlobStore::getBlob()
+                                               // Ignore $setOpts; blobs are immutable and negatives are not cached
                                                $blob = ExternalStore::fetchFromURL( $url, [ 'wiki' => $this->wikiId ] );
 
                                                return $blob === false ? false : $this->decompressData( $blob, $flags );
index 17c8040..455ff45 100644 (file)
@@ -169,7 +169,6 @@ class ApiStashEdit extends ApiBase {
         * @since 1.25
         */
        public static function parseAndStash( WikiPage $page, Content $content, User $user, $summary ) {
-               $cache = ObjectCache::getLocalClusterInstance();
                $logger = LoggerFactory::getInstance( 'StashEdit' );
 
                $title = $page->getTitle();
@@ -195,7 +194,7 @@ class ApiStashEdit extends ApiBase {
                $cutoffTime = time() - self::PRESUME_FRESH_TTL_SEC;
 
                // Reuse any freshly build matching edit stash cache
-               $editInfo = $cache->get( $key );
+               $editInfo = self::getStashValue( $key );
                if ( $editInfo && wfTimestamp( TS_UNIX, $editInfo->timestamp ) >= $cutoffTime ) {
                        $alreadyCached = true;
                } else {
@@ -216,29 +215,27 @@ class ApiStashEdit extends ApiBase {
                                return self::ERROR_NONE;
                        }
 
-                       list( $stashInfo, $ttl, $code ) = self::buildStashValue(
+                       $code = self::storeStashValue(
+                               $key,
                                $editInfo->pstContent,
                                $editInfo->output,
                                $editInfo->timestamp,
                                $user
                        );
 
-                       if ( $stashInfo ) {
-                               $ok = $cache->set( $key, $stashInfo, $ttl );
-                               if ( $ok ) {
-                                       $logger->debug( "Cached parser output for key '{cachekey}' ('{title}').",
-                                               [ 'cachekey' => $key, 'title' => $titleStr ] );
-                                       return self::ERROR_NONE;
-                               } else {
-                                       $logger->error( "Failed to cache parser output for key '{cachekey}' ('{title}').",
-                                               [ 'cachekey' => $key, 'title' => $titleStr ] );
-                                       return self::ERROR_CACHE;
-                               }
-                       } else {
-                               // @todo Doesn't seem reachable, see @todo in buildStashValue
-                               $logger->info( "Uncacheable parser output for key '{cachekey}' ('{title}') [{code}].",
+                       if ( $code === true ) {
+                               $logger->debug( "Cached parser output for key '{cachekey}' ('{title}').",
+                                       [ 'cachekey' => $key, 'title' => $titleStr ] );
+                               return self::ERROR_NONE;
+                       } elseif ( $code === 'uncacheable' ) {
+                               $logger->info(
+                                       "Uncacheable parser output for key '{cachekey}' ('{title}') [{code}].",
                                        [ 'cachekey' => $key, 'title' => $titleStr, 'code' => $code ] );
                                return self::ERROR_UNCACHEABLE;
+                       } else {
+                               $logger->error( "Failed to cache parser output for key '{cachekey}' ('{title}').",
+                                       [ 'cachekey' => $key, 'title' => $titleStr, 'code' => $code ] );
+                               return self::ERROR_CACHE;
                        }
                }
 
@@ -267,12 +264,11 @@ class ApiStashEdit extends ApiBase {
                        return false; // bots never stash - don't pollute stats
                }
 
-               $cache = ObjectCache::getLocalClusterInstance();
                $logger = LoggerFactory::getInstance( 'StashEdit' );
                $stats = MediaWikiServices::getInstance()->getStatsdDataFactory();
 
                $key = self::getStashKey( $title, self::getContentHash( $content ), $user );
-               $editInfo = $cache->get( $key );
+               $editInfo = self::getStashValue( $key );
                if ( !is_object( $editInfo ) ) {
                        $start = microtime( true );
                        // We ignore user aborts and keep parsing. Block on any prior parsing
@@ -282,7 +278,7 @@ class ApiStashEdit extends ApiBase {
                        $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
                        $dbw = $lb->getAnyOpenConnection( $lb->getWriterIndex() );
                        if ( $dbw && $dbw->lock( $key, __METHOD__, 30 ) ) {
-                               $editInfo = $cache->get( $key );
+                               $editInfo = self::getStashValue( $key );
                                $dbw->unlock( $key, __METHOD__ );
                        }
 
@@ -378,7 +374,7 @@ class ApiStashEdit extends ApiBase {
         */
        private static function getStashKey( Title $title, $contentHash, User $user ) {
                return ObjectCache::getLocalClusterInstance()->makeKey(
-                       'prepared-edit',
+                       'stashed-edit-info',
                        md5( $title->getPrefixedDBkey() ),
                        // Account for the edit model/text
                        $contentHash,
@@ -387,46 +383,85 @@ class ApiStashEdit extends ApiBase {
                );
        }
 
+       /**
+        * @param string $uuid
+        * @return string
+        */
+       private static function getStashParserOutputKey( $uuid ) {
+               return ObjectCache::getLocalClusterInstance()->makeKey( 'stashed-edit-output', $uuid );
+       }
+
+       /**
+        * @param string $key
+        * @return stdClass|bool Object map (pstContent,output,outputID,timestamp,edits) or false
+        */
+       private static function getStashValue( $key ) {
+               $cache = ObjectCache::getLocalClusterInstance();
+
+               $stashInfo = $cache->get( $key );
+               if ( !is_object( $stashInfo ) ) {
+                       return false;
+               }
+
+               $parserOutputKey = self::getStashParserOutputKey( $stashInfo->outputID );
+               $parserOutput = $cache->get( $parserOutputKey );
+               if ( $parserOutput instanceof ParserOutput ) {
+                       $stashInfo->output = $parserOutput;
+
+                       return $stashInfo;
+               }
+
+               return false;
+       }
+
        /**
         * Build a value to store in memcached based on the PST content and parser output
         *
         * This makes a simple version of WikiPage::prepareContentForEdit() as stash info
         *
+        * @param string $key
         * @param Content $pstContent Pre-Save transformed content
         * @param ParserOutput $parserOutput
         * @param string $timestamp TS_MW
         * @param User $user
-        * @return array (stash info array, TTL in seconds, info code) or (null, 0, info code)
+        * @return string|bool True or an error code
         */
-       private static function buildStashValue(
-               Content $pstContent, ParserOutput $parserOutput, $timestamp, User $user
+       private static function storeStashValue(
+               $key, Content $pstContent, ParserOutput $parserOutput, $timestamp, User $user
        ) {
                // If an item is renewed, mind the cache TTL determined by config and parser functions.
                // Put an upper limit on the TTL for sanity to avoid extreme template/file staleness.
-               $since = time() - wfTimestamp( TS_UNIX, $parserOutput->getCacheTime() );
-               $ttl = min( $parserOutput->getCacheExpiry() - $since, self::MAX_CACHE_TTL );
-
+               $age = time() - wfTimestamp( TS_UNIX, $parserOutput->getCacheTime() );
+               $ttl = min( $parserOutput->getCacheExpiry() - $age, self::MAX_CACHE_TTL );
                // Avoid extremely stale user signature timestamps (T84843)
                if ( $parserOutput->getFlag( 'user-signature' ) ) {
                        $ttl = min( $ttl, self::MAX_SIGNATURE_TTL );
                }
 
                if ( $ttl <= 0 ) {
-                       // @todo It doesn't seem like this can occur, because it would mean an entry older than
-                       // getCacheExpiry() seconds, which is much longer than PRESUME_FRESH_TTL_SEC, and
-                       // anything older than PRESUME_FRESH_TTL_SEC will have been thrown out already.
-                       return [ null, 0, 'no_ttl' ];
+                       return 'uncacheable'; // low TTL due to a tag, magic word, or signature?
                }
 
-               // Only store what is actually needed
+               // Store what is actually needed and split the output into another key (T204742)
+               $parseroutputID = md5( $key );
                $stashInfo = (object)[
                        'pstContent' => $pstContent,
-                       'output'     => $parserOutput,
+                       'outputID'   => $parseroutputID,
                        'timestamp'  => $timestamp,
                        'edits'      => $user->getEditCount()
                ];
 
-               return [ $stashInfo, $ttl, 'ok' ];
+               $cache = ObjectCache::getLocalClusterInstance();
+               $ok = $cache->set( $key, $stashInfo, $ttl );
+               if ( $ok ) {
+                       $ok = $cache->set(
+                               self::getStashParserOutputKey( $parseroutputID ),
+                               $parserOutput,
+                               $ttl
+                       );
+               }
+
+               return $ok ? true : 'store_error';
        }
 
        public function getAllowedParams() {
index e41c3a2..1fedfaf 100644 (file)
@@ -104,7 +104,7 @@ class APCBagOStuff extends BagOStuff {
                return $value;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                apc_delete( $key . self::KEY_SUFFIX );
 
                return true;
index a26e560..fb43d4d 100644 (file)
@@ -55,7 +55,7 @@ class APCUBagOStuff extends APCBagOStuff {
                return true;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                apcu_delete( $key . self::KEY_SUFFIX );
 
                return true;
index 44c79f3..a3c020e 100644 (file)
@@ -262,8 +262,9 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         *
         * @param string $key
         * @return bool True if the item was deleted or not found, false on failure
+        * @param int $flags Bitfield of BagOStuff::WRITE_* constants
         */
-       abstract public function delete( $key );
+       abstract public function delete( $key, $flags = 0 );
 
        /**
         * Merge changes into the existing cache value (possibly creating a new one)
@@ -710,7 +711,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         * The callbacks may or may not be called ever, in any particular order.
         * They are likely to be invoked when something WRITE_SYNC is used used.
         * They should follow a caching pattern as shown below, so that any code
-        * using the word will get it's result no matter what happens.
+        * using the work will get it's result no matter what happens.
         * @code
         *     $result = null;
         *     $workCallback = function () use ( &$result ) {
index 726836e..3927ed3 100644 (file)
@@ -68,7 +68,7 @@ class CachedBagOStuff extends HashBagOStuff {
        }
 
        public function delete( $key, $flags = 0 ) {
-               parent::delete( $key );
+               parent::delete( $key, $flags );
                if ( !( $flags & self::WRITE_CACHE_ONLY ) ) {
                        $this->backend->delete( $key );
                }
index 3f66c06..3bf58df 100644 (file)
@@ -39,7 +39,7 @@ class EmptyBagOStuff extends BagOStuff {
                return true;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                return true;
        }
 
index ec66492..7d074fa 100644 (file)
@@ -106,7 +106,7 @@ class HashBagOStuff extends BagOStuff {
                return true;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                unset( $this->bag[$key] );
 
                return true;
index f7bf86b..bf0da11 100644 (file)
@@ -70,7 +70,7 @@ class MemcachedBagOStuff extends BagOStuff {
                        $value, $this->fixExpiry( $exptime ) );
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                return $this->client->delete( $this->validateKeyEncoding( $key ) );
        }
 
index fe31c25..a6646bc 100644 (file)
@@ -172,7 +172,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
                return $this->checkResult( $key, parent::cas( $casToken, $key, $value, $exptime ) );
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                $this->debugLog( "delete($key)" );
                $result = parent::delete( $key );
                if ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTFOUND ) {
index 043f8cb..f549876 100644 (file)
@@ -139,7 +139,7 @@ class MultiWriteBagOStuff extends BagOStuff {
                return $this->doWrite( $this->cacheIndexes, $asyncWrites, 'set', $key, $value, $exptime );
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'delete', $key );
        }
 
index fc3d575..b0b82d8 100644 (file)
@@ -124,16 +124,8 @@ class RESTBagOStuff extends BagOStuff {
                return false;
        }
 
-       /**
-        * Set an item
-        *
-        * @param string $key
-        * @param mixed $value
-        * @param int $exptime Either an interval in seconds or a unix timestamp for expiry
-        * @param int $flags Bitfield of BagOStuff::WRITE_* constants
-        * @return bool Success
-        */
        public function set( $key, $value, $exptime = 0, $flags = 0 ) {
+               // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
                $req = [
                        'method' => 'PUT',
                        'url' => $this->url . rawurlencode( $key ),
@@ -146,13 +138,8 @@ class RESTBagOStuff extends BagOStuff {
                return $this->handleError( "Failed to store $key", $rcode, $rerr );
        }
 
-       /**
-        * Delete an item.
-        *
-        * @param string $key
-        * @return bool True if the item was deleted or not found, false on failure
-        */
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
+               // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
                $req = [
                        'method' => 'DELETE',
                        'url' => $this->url . rawurlencode( $key ),
index abf9e8b..a0febfc 100644 (file)
@@ -132,7 +132,7 @@ class RedisBagOStuff extends BagOStuff {
                return $result;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                list( $server, $conn ) = $this->getConnection( $key );
                if ( !$conn ) {
                        return false;
index f2c3732..e2b9a52 100644 (file)
@@ -90,8 +90,8 @@ class ReplicatedBagOStuff extends BagOStuff {
                return $this->writeStore->set( $key, $value, $exptime, $flags );
        }
 
-       public function delete( $key ) {
-               return $this->writeStore->delete( $key );
+       public function delete( $key, $flags = 0 ) {
+               return $this->writeStore->delete( $key, $flags );
        }
 
        public function add( $key, $value, $exptime = 0 ) {
index 764230b..6b0bec0 100644 (file)
@@ -45,7 +45,7 @@ class WinCacheBagOStuff extends BagOStuff {
                return ( is_array( $result ) && $result === [] ) || $result;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                wincache_ucache_delete( $key );
 
                return true;
index 74abace..87cc2bd 100644 (file)
@@ -22,8 +22,7 @@
 namespace Wikimedia\Rdbms;
 
 /**
- * Exception class for replica DB wait timeouts
- * @deprecated since 1.32
+ * Exception class for replica DB wait errors
  * @ingroup Database
  */
 class DBReplicationWaitError extends DBExpectedError {
index 2866a6c..eeb7355 100644 (file)
@@ -423,7 +423,9 @@ class SqlBagOStuff extends BagOStuff {
                return (bool)$db->affectedRows();
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
+               $ok = true;
+
                list( $serverIndex, $tableName ) = $this->getTableByKey( $key );
                $db = null;
                $silenceScope = $this->silenceTransactionProfiler();
@@ -435,10 +437,13 @@ class SqlBagOStuff extends BagOStuff {
                                __METHOD__ );
                } catch ( DBError $e ) {
                        $this->handleWriteError( $e, $db, $serverIndex );
-                       return false;
+                       $ok = false;
+               }
+               if ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC ) {
+                       $ok = $this->waitForReplication() && $ok;
                }
 
-               return true;
+               return $ok;
        }
 
        public function incr( $key, $step = 1 ) {
@@ -711,14 +716,8 @@ class SqlBagOStuff extends BagOStuff {
                if ( $exception instanceof DBConnectionError ) {
                        $this->markServerDown( $exception, $serverIndex );
                }
-               $this->logger->error( "DBError: {$exception->getMessage()}" );
-               if ( $exception instanceof DBConnectionError ) {
-                       $this->setLastError( BagOStuff::ERR_UNREACHABLE );
-                       $this->logger->debug( __METHOD__ . ": ignoring connection error" );
-               } else {
-                       $this->setLastError( BagOStuff::ERR_UNEXPECTED );
-                       $this->logger->debug( __METHOD__ . ": ignoring query error" );
-               }
+
+               $this->setAndLogDBError( $exception );
        }
 
        /**
@@ -732,15 +731,15 @@ class SqlBagOStuff extends BagOStuff {
        protected function handleWriteError( DBError $exception, IDatabase $db = null, $serverIndex ) {
                if ( !$db ) {
                        $this->markServerDown( $exception, $serverIndex );
-               } elseif ( $db->wasReadOnlyError() ) {
-                       if ( $db->trxLevel() && $this->usesMainDB() ) {
-                               // Errors like deadlocks and connection drops already cause rollback.
-                               // For consistency, we have no choice but to throw an error and trigger
-                               // complete rollback if the main DB is also being used as the cache DB.
-                               throw $exception;
-                       }
                }
 
+               $this->setAndLogDBError( $exception );
+       }
+
+       /**
+        * @param DBError $exception
+        */
+       private function setAndLogDBError( DBError $exception ) {
                $this->logger->error( "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
                        $this->setLastError( BagOStuff::ERR_UNREACHABLE );
@@ -813,20 +812,26 @@ class SqlBagOStuff extends BagOStuff {
                }
 
                // Main LB is used; wait for any replica DBs to catch up
-               $masterPos = $lb->getMasterPos();
-               if ( !$masterPos ) {
-                       return true; // not applicable
-               }
+               try {
+                       $masterPos = $lb->getMasterPos();
+                       if ( !$masterPos ) {
+                               return true; // not applicable
+                       }
 
-               $loop = new WaitConditionLoop(
-                       function () use ( $lb, $masterPos ) {
-                               return $lb->waitForAll( $masterPos, 1 );
-                       },
-                       $this->syncTimeout,
-                       $this->busyCallbacks
-               );
+                       $loop = new WaitConditionLoop(
+                               function () use ( $lb, $masterPos ) {
+                                       return $lb->waitForAll( $masterPos, 1 );
+                               },
+                               $this->syncTimeout,
+                               $this->busyCallbacks
+                       );
 
-               return ( $loop->invoke() === $loop::CONDITION_REACHED );
+                       return ( $loop->invoke() === $loop::CONDITION_REACHED );
+               } catch ( DBError $e ) {
+                       $this->setAndLogDBError( $e );
+
+                       return false;
+               }
        }
 
        /**
index ce1d2d0..64dbb22 100644 (file)
@@ -67,21 +67,23 @@ use Wikimedia\Rdbms\IDatabase;
  * @ingroup Pager
  */
 abstract class IndexPager extends ContextSource implements Pager {
-       /**
-        * Constants for the $mDefaultDirection field.
-        *
-        * These are boolean for historical reasons and should stay boolean for backwards-compatibility.
-        */
+       /** Backwards-compatible constant for $mDefaultDirection field (do not change) */
        const DIR_ASCENDING = false;
+       /** Backwards-compatible constant for $mDefaultDirection field (do not change) */
        const DIR_DESCENDING = true;
 
+       /** Backwards-compatible constant for reallyDoQuery() (do not change) */
+       const QUERY_ASCENDING = true;
+       /** Backwards-compatible constant for reallyDoQuery() (do not change) */
+       const QUERY_DESCENDING = false;
+
        /** @var WebRequest */
        public $mRequest;
        /** @var int[] List of default entry limit options to be presented to clients */
        public $mLimitsShown = [ 20, 50, 100, 250, 500 ];
        /** @var int The default entry limit choosen for clients */
        public $mDefaultLimit = 50;
-       /** @var string|int The starting point to enumerate entries */
+       /** @var mixed The starting point to enumerate entries */
        public $mOffset;
        /** @var int The maximum number of entries to show */
        public $mLimit;
@@ -89,20 +91,23 @@ abstract class IndexPager extends ContextSource implements Pager {
        public $mQueryDone = false;
        /** @var IDatabase */
        public $mDb;
-       /** @var stdClass|null Extra row fetched at the end to see if the end was reached */
+       /** @var stdClass|bool|null Extra row fetched at the end to see if the end was reached */
        public $mPastTheEndRow;
 
        /**
         * The index to actually be used for ordering. This is a single column,
         * for one ordering, even if multiple orderings are supported.
+        * @var string
         */
        protected $mIndexField;
        /**
         * An array of secondary columns to order by. These fields are not part of the offset.
         * This is a column list for one ordering, even if multiple orderings are supported.
+        * @var string[]
         */
        protected $mExtraSortFields;
        /** For pages that support multiple types of ordering, which one to use.
+        * @var string|null
         */
        protected $mOrderType;
        /**
@@ -115,18 +120,31 @@ abstract class IndexPager extends ContextSource implements Pager {
         *
         * Like $mIndexField, $mDefaultDirection will be a single value even if the
         * class supports multiple default directions for different order types.
+        * @var bool
         */
        public $mDefaultDirection;
+       /** @var bool */
        public $mIsBackwards;
 
-       /** True if the current result set is the first one */
+       /** @var bool True if the current result set is the first one */
        public $mIsFirst;
+       /** @var bool */
        public $mIsLast;
 
-       protected $mLastShown, $mFirstShown, $mPastTheEndIndex, $mDefaultQuery, $mNavigationBar;
+       /** @var mixed */
+       protected $mLastShown;
+       /** @var mixed */
+       protected $mFirstShown;
+       /** @var mixed */
+       protected $mPastTheEndIndex;
+       /** @var array */
+       protected $mDefaultQuery;
+       /** @var string */
+       protected $mNavigationBar;
 
        /**
         * Whether to include the offset in the query
+        * @var bool
         */
        protected $mIncludeOffset = false;
 
@@ -208,11 +226,13 @@ abstract class IndexPager extends ContextSource implements Pager {
        public function doQuery() {
                # Use the child class name for profiling
                $fname = __METHOD__ . ' (' . static::class . ')';
+               /** @noinspection PhpUnusedLocalVariableInspection */
                $section = Profiler::instance()->scopedProfileIn( $fname );
 
-               $descending = $this->mIsBackwards
-                       ? ( $this->mDefaultDirection === self::DIR_DESCENDING )
-                       : ( $this->mDefaultDirection === self::DIR_ASCENDING );
+               $defaultOrder = ( $this->mDefaultDirection === self::DIR_ASCENDING )
+                       ? self::QUERY_ASCENDING
+                       : self::QUERY_DESCENDING;
+               $order = $this->mIsBackwards ? self::oppositeOrder( $defaultOrder ) : $defaultOrder;
 
                # Plus an extra row so that we can tell the "next" link should be shown
                $queryLimit = $this->mLimit + 1;
@@ -225,14 +245,15 @@ abstract class IndexPager extends ContextSource implements Pager {
                        // direction see if we get a row.
                        $oldIncludeOffset = $this->mIncludeOffset;
                        $this->mIncludeOffset = !$this->mIncludeOffset;
-                       $isFirst = !$this->reallyDoQuery( $this->mOffset, 1, !$descending )->numRows();
+                       $oppositeOrder = self::oppositeOrder( $order );
+                       $isFirst = !$this->reallyDoQuery( $this->mOffset, 1, $oppositeOrder )->numRows();
                        $this->mIncludeOffset = $oldIncludeOffset;
                }
 
                $this->mResult = $this->reallyDoQuery(
                        $this->mOffset,
                        $queryLimit,
-                       $descending
+                       $order
                );
 
                $this->extractResultInfo( $isFirst, $queryLimit, $this->mResult );
@@ -242,6 +263,16 @@ abstract class IndexPager extends ContextSource implements Pager {
                $this->mResult->rewind(); // Paranoia
        }
 
+       /**
+        * @param bool $order One of the IndexPager::QUERY_* class constants
+        * @return bool The opposite query order as an IndexPager::QUERY_ constant
+        */
+       final protected static function oppositeOrder( $order ) {
+               return ( $order === self::QUERY_ASCENDING )
+                       ? self::QUERY_DESCENDING
+                       : self::QUERY_ASCENDING;
+       }
+
        /**
         * @return IResultWrapper The result wrapper.
         */
@@ -364,17 +395,18 @@ abstract class IndexPager extends ContextSource implements Pager {
        }
 
        /**
-        * Do a query with specified parameters, rather than using the object
-        * context
+        * Do a query with specified parameters, rather than using the object context
+        *
+        * @note For b/c, query direction is true for ascending and false for descending
         *
         * @param string $offset Index offset, inclusive
         * @param int $limit Exact query limit
-        * @param bool $descending Query direction, false for ascending, true for descending
+        * @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
         * @return IResultWrapper
         */
-       public function reallyDoQuery( $offset, $limit, $descending ) {
+       public function reallyDoQuery( $offset, $limit, $order ) {
                list( $tables, $fields, $conds, $fname, $options, $join_conds ) =
-                       $this->buildQueryInfo( $offset, $limit, $descending );
+                       $this->buildQueryInfo( $offset, $limit, $order );
 
                return $this->mDb->select( $tables, $fields, $conds, $fname, $options, $join_conds );
        }
@@ -382,12 +414,14 @@ abstract class IndexPager extends ContextSource implements Pager {
        /**
         * Build variables to use by the database wrapper.
         *
+        * @note For b/c, query direction is true for ascending and false for descending
+        *
         * @param string $offset Index offset, inclusive
         * @param int $limit Exact query limit
-        * @param bool $descending Query direction, false for ascending, true for descending
+        * @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
         * @return array
         */
-       protected function buildQueryInfo( $offset, $limit, $descending ) {
+       protected function buildQueryInfo( $offset, $limit, $order ) {
                $fname = __METHOD__ . ' (' . $this->getSqlComment() . ')';
                $info = $this->getQueryInfo();
                $tables = $info['tables'];
@@ -396,7 +430,7 @@ abstract class IndexPager extends ContextSource implements Pager {
                $options = $info['options'] ?? [];
                $join_conds = $info['join_conds'] ?? [];
                $sortColumns = array_merge( [ $this->mIndexField ], $this->mExtraSortFields );
-               if ( $descending ) {
+               if ( $order === self::QUERY_ASCENDING ) {
                        $options['ORDER BY'] = $sortColumns;
                        $operator = $this->mIncludeOffset ? '>=' : '>';
                } else {
index bf36983..5caf1f4 100644 (file)
@@ -26,6 +26,7 @@ use Wikimedia\Timestamp\TimestampException;
  */
 abstract class RangeChronologicalPager extends ReverseChronologicalPager {
 
+       /** @var string[] */
        protected $rangeConds = [];
 
        /**
@@ -93,14 +94,14 @@ abstract class RangeChronologicalPager extends ReverseChronologicalPager {
         *
         * @param string $offset Index offset, inclusive
         * @param int $limit Exact query limit
-        * @param bool $descending Query direction, false for ascending, true for descending
+        * @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
         * @return array
         */
-       protected function buildQueryInfo( $offset, $limit, $descending ) {
+       protected function buildQueryInfo( $offset, $limit, $order ) {
                list( $tables, $fields, $conds, $fname, $options, $join_conds ) = parent::buildQueryInfo(
                        $offset,
                        $limit,
-                       $descending
+                       $order
                );
 
                if ( $this->rangeConds ) {
index 51824d2..e8f3f40 100644 (file)
@@ -26,9 +26,13 @@ use Wikimedia\Timestamp\TimestampException;
  * @ingroup Pager
  */
 abstract class ReverseChronologicalPager extends IndexPager {
+       /** @var bool */
        public $mDefaultDirection = IndexPager::DIR_DESCENDING;
+       /** @var int */
        public $mYear;
+       /** @var int */
        public $mMonth;
+       /** @var int */
        public $mDay;
 
        public function getNavigationBar() {
index 8934fc2..f6445f5 100644 (file)
  * @ingroup Pager
  */
 abstract class TablePager extends IndexPager {
+       /** @var string */
        protected $mSort;
 
+       /** @var stdClass */
        protected $mCurrentRow;
 
        public function __construct( IContextSource $context = null ) {
index 3e1a869..5dbdba0 100644 (file)
@@ -161,11 +161,11 @@ class ActiveUsersPager extends UsersPager {
                ];
        }
 
-       protected function buildQueryInfo( $offset, $limit, $descending ) {
+       protected function buildQueryInfo( $offset, $limit, $order ) {
                $fname = __METHOD__ . ' (' . $this->getSqlComment() . ')';
 
                $sortColumns = array_merge( [ $this->mIndexField ], $this->mExtraSortFields );
-               if ( $descending ) {
+               if ( $order === self::QUERY_ASCENDING ) {
                        $dir = 'ASC';
                        $orderBy = $sortColumns;
                        $operator = $this->mIncludeOffset ? '>=' : '>';
index f045333..8120417 100644 (file)
@@ -167,24 +167,25 @@ class AllMessagesTablePager extends TablePager {
        }
 
        /**
-        *  This function normally does a database query to get the results; we need
+        * This function normally does a database query to get the results; we need
         * to make a pretend result using a FakeResultWrapper.
         * @param string $offset
         * @param int $limit
-        * @param bool $descending
+        * @param bool $order
         * @return FakeResultWrapper
         */
-       function reallyDoQuery( $offset, $limit, $descending ) {
+       function reallyDoQuery( $offset, $limit, $order ) {
+               $asc = ( $order === self::QUERY_ASCENDING );
                $result = new FakeResultWrapper( [] );
 
-               $messageNames = $this->getAllMessages( $descending );
+               $messageNames = $this->getAllMessages( $order );
                $statuses = self::getCustomisedStatuses( $messageNames, $this->langcode, $this->foreign );
 
                $count = 0;
                foreach ( $messageNames as $key ) {
                        $customised = isset( $statuses['pages'][$key] );
                        if ( $customised !== $this->custom &&
-                               ( $descending && ( $key < $offset || !$offset ) || !$descending && $key > $offset ) &&
+                               ( $asc && ( $key < $offset || !$offset ) || !$asc && $key > $offset ) &&
                                ( ( $this->prefix && preg_match( $this->prefix, $key ) ) || $this->prefix === false )
                        ) {
                                $actual = wfMessage( $key )->inLanguage( $this->lang )->plain();
index 1220e86..382ba2f 100644 (file)
@@ -160,14 +160,14 @@ class ContribsPager extends RangeChronologicalPager {
         *
         * @param string $offset Index offset, inclusive
         * @param int $limit Exact query limit
-        * @param bool $descending Query direction, false for ascending, true for descending
+        * @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
         * @return IResultWrapper
         */
-       function reallyDoQuery( $offset, $limit, $descending ) {
+       function reallyDoQuery( $offset, $limit, $order ) {
                list( $tables, $fields, $conds, $fname, $options, $join_conds ) = $this->buildQueryInfo(
                        $offset,
                        $limit,
-                       $descending
+                       $order
                );
 
                /*
@@ -193,7 +193,7 @@ class ContribsPager extends RangeChronologicalPager {
                ) ];
                Hooks::run(
                        'ContribsPager::reallyDoQuery',
-                       [ &$data, $this, $offset, $limit, $descending ]
+                       [ &$data, $this, $offset, $limit, $order ]
                );
 
                $result = [];
@@ -207,7 +207,7 @@ class ContribsPager extends RangeChronologicalPager {
                }
 
                // sort results
-               if ( $descending ) {
+               if ( $order === self::QUERY_ASCENDING ) {
                        ksort( $result );
                } else {
                        krsort( $result );
index fa0fbf3..56b799b 100644 (file)
@@ -115,17 +115,17 @@ class DeletedContribsPager extends IndexPager {
         *
         * @param string $offset Index offset, inclusive
         * @param int $limit Exact query limit
-        * @param bool $descending Query direction, false for ascending, true for descending
+        * @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
         * @return IResultWrapper
         */
-       function reallyDoQuery( $offset, $limit, $descending ) {
-               $data = [ parent::reallyDoQuery( $offset, $limit, $descending ) ];
+       function reallyDoQuery( $offset, $limit, $order ) {
+               $data = [ parent::reallyDoQuery( $offset, $limit, $order ) ];
 
                // This hook will allow extensions to add in additional queries, nearly
                // identical to ContribsPager::reallyDoQuery.
                Hooks::run(
                        'DeletedContribsPager::reallyDoQuery',
-                       [ &$data, $this, $offset, $limit, $descending ]
+                       [ &$data, $this, $offset, $limit, $order ]
                );
 
                $result = [];
@@ -139,7 +139,7 @@ class DeletedContribsPager extends IndexPager {
                }
 
                // sort results
-               if ( $descending ) {
+               if ( $order === self::QUERY_ASCENDING ) {
                        ksort( $result );
                } else {
                        krsort( $result );
index 1794362..486b0ec 100644 (file)
@@ -321,15 +321,15 @@ class ImageListPager extends TablePager {
         *   is descending, so I renamed it to $asc here.
         * @param int $offset
         * @param int $limit
-        * @param bool $asc
-        * @return array
+        * @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
+        * @return FakeResultWrapper
         * @throws MWException
         */
-       function reallyDoQuery( $offset, $limit, $asc ) {
+       function reallyDoQuery( $offset, $limit, $order ) {
                $prevTableName = $this->mTableName;
                $this->mTableName = 'image';
                list( $tables, $fields, $conds, $fname, $options, $join_conds ) =
-                       $this->buildQueryInfo( $offset, $limit, $asc );
+                       $this->buildQueryInfo( $offset, $limit, $order );
                $imageRes = $this->mDb->select( $tables, $fields, $conds, $fname, $options, $join_conds );
                $this->mTableName = $prevTableName;
 
@@ -347,13 +347,13 @@ class ImageListPager extends TablePager {
                $this->mIndexField = 'oi_' . substr( $this->mIndexField, 4 );
 
                list( $tables, $fields, $conds, $fname, $options, $join_conds ) =
-                       $this->buildQueryInfo( $offset, $limit, $asc );
+                       $this->buildQueryInfo( $offset, $limit, $order );
                $oldimageRes = $this->mDb->select( $tables, $fields, $conds, $fname, $options, $join_conds );
 
                $this->mTableName = $prevTableName;
                $this->mIndexField = $oldIndex;
 
-               return $this->combineResult( $imageRes, $oldimageRes, $limit, $asc );
+               return $this->combineResult( $imageRes, $oldimageRes, $limit, $order );
        }
 
        /**
index 5ef3b04..a63f8aa 100644 (file)
@@ -331,6 +331,8 @@ class ApiStashEditTest extends ApiTestCase {
                $cache = ObjectCache::getLocalClusterInstance();
 
                $editInfo = $cache->get( $key );
+               $outputKey = $cache->makeKey( 'stashed-edit-output', $editInfo->outputID );
+               $editInfo->output = $cache->get( $outputKey );
                $editInfo->output->setCacheTime( wfTimestamp( TS_MW,
                        wfTimestamp( TS_UNIX, $editInfo->output->getCacheTime() ) - $howOld - 1 ) );
 
index dad7bf2..164b466 100644 (file)
@@ -1472,14 +1472,18 @@ class UserTest extends MediaWikiTestCase {
                MWTimestamp::setFakeTime( function () use ( &$clock ) {
                        return $clock += 1000;
                } );
-               $user = $this->getTestUser()->getUser();
-               $firstRevision = self::makeEdit( $user, 'Help:UserTest_GetEditTimestamp', 'one', 'test' );
-               $secondRevision = self::makeEdit( $user, 'Help:UserTest_GetEditTimestamp', 'two', 'test' );
-               // Sanity check: revisions timestamp are different
-               $this->assertNotEquals( $firstRevision->getTimestamp(), $secondRevision->getTimestamp() );
-
-               $this->assertEquals( $firstRevision->getTimestamp(), $user->getFirstEditTimestamp() );
-               $this->assertEquals( $secondRevision->getTimestamp(), $user->getLatestEditTimestamp() );
+               try {
+                       $user = $this->getTestUser()->getUser();
+                       $firstRevision = self::makeEdit( $user, 'Help:UserTest_GetEditTimestamp', 'one', 'test' );
+                       $secondRevision = self::makeEdit( $user, 'Help:UserTest_GetEditTimestamp', 'two', 'test' );
+                       // Sanity check: revisions timestamp are different
+                       $this->assertNotEquals( $firstRevision->getTimestamp(), $secondRevision->getTimestamp() );
+
+                       $this->assertEquals( $firstRevision->getTimestamp(), $user->getFirstEditTimestamp() );
+                       $this->assertEquals( $secondRevision->getTimestamp(), $user->getLatestEditTimestamp() );
+               } finally {
+                       MWTimestamp::setFakeTime( false );
+               }
        }
 
        /**